Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Android 4.1-4.3 WebView source baseUrl bug #13789

Closed
wants to merge 2 commits into from

Conversation

Etheryte
Copy link
Contributor

@Etheryte Etheryte commented May 5, 2017

Motivation

Resolves #11753 and #9835.

Description

Android versions 4.1-4.3 don't understand the MIME type text/html; charset=utf-8 and default to text/plain instead, rendering the content as an unparsed HTML string. Since the encoding is already set and passed separately, removing it from the MIME type has no negative effects.

The same fix has already been discussed, successfully tested and incorporated in NativeScript/NativeScript#1038.

Resolves facebook#11753. Android versions 4.1-4.3 don't understand the MIME type 'text/html; charset=utf-8' and default to 'text/plain' instead, rendering the content as an unparsed HTML string. Since the encoding is already set and passed separately, removing it from the MIME type has no negative effects.
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed labels May 5, 2017
Etheryte pushed a commit to Etheryte/react-native that referenced this pull request May 14, 2017
@shergin shergin added the Android label Jun 2, 2017
Etheryte pushed a commit to Etheryte/react-native that referenced this pull request Jun 7, 2017
@Exilz
Copy link

Exilz commented Jul 3, 2017

Can someone review this PR so we can see it merged in the near future ? We had to make the exact same change on react-native, this is a quite critical bug for us.

@stereodenis
Copy link
Contributor

@mkonicek how to help you get it merged?

@chung2014
Copy link

will this fix merge to master?

@vikkalen
Copy link

the scope of the property HTML_MIME_TYPE has recently changed from "private" to "protected". See commit e2c87b5. You should probably fix your pull request.

@facebook-github-bot
Copy link
Contributor

@Etheryte I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

@Etheryte
Copy link
Contributor Author

@facebook-github-bot This pull request is still relevant, I've merged master into the branch. I don't know who to highlight with this request.

@pull-bot
Copy link

Warnings
⚠️

📋 Test Plan - This PR appears to be missing a Test Plan.

@facebook-github-bot label Needs more information

Generated by 🚫 dangerJS

Etheryte pushed a commit to Etheryte/react-native that referenced this pull request Sep 25, 2017
@facebook-github-bot
Copy link
Contributor

@Etheryte I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

@chirag04
Copy link
Contributor

cc @shergin

@shergin
Copy link
Contributor

shergin commented Sep 29, 2017

I can merge this on Monday.

@Crash--
Copy link
Contributor

Crash-- commented Oct 3, 2017

+1 to merge this PR. ATM, we've to override the setSource() methods in order to be able to display HTML on Android 4.2.2 :(

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Oct 4, 2017
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Android4.1-4.3][v0.39]WebView source={{html}} show all html string without parse html if 'baseUrl' is set
10 participants